.aiox-core/core/resilience/agent-immortality.js (3)

119-137: 💤 Low value

Cause-of-death heuristic ordering looks intentional — please confirm.

The order matters because text aggregates name+message+stack and several keywords overlap (e.g., a tool failure whose stack mentions length/tokens would classify as CONTEXT_OVERFLOW; a tool error caused by an HTTP timeout would classify as TOOL_EXECUTION_FAILURE before reaching the API branch because of the tool keyword). The current priority OVERFLOW > RECURSIVE > TOOL > API > UNKNOWN is defensible, but worth a comment to justify the ordering and prevent future "fixes" that reorder it. Minor: the regex \b(...|enoent|...) matches case-insensitively on the lowercased text, so listing enoent/econn works — just noting in case anyone later removes .toLowerCase().

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.aiox-core/core/resilience/agent-immortality.js around lines 119 - 137, Add
an explicit comment above the diagnoseCause function explaining that the
heuristic ordering (CONTEXT_OVERFLOW > RECURSIVE_LOOP > TOOL_EXECUTION_FAILURE >
EXTERNAL_API_ERROR > UNKNOWN) is intentional to prioritize
context/token/overflow signals over recursive/tool/api signals because keywords
can overlap in name/message/stack aggregation; also note that text is lowercased
so regex entries like "enoent" and "econn" rely on that and should remain
lowercase if the toLowerCase() call is kept. This will prevent future accidental
reordering and clarify why TOOL and API checks come after overflow/recursive
checks.

---

52-57: ⚡ Quick win

nextId collisions across processes.

idCounter is module-scoped, so two processes started in the same millisecond will produce identical autopsy-…, state-…, and reincarnation-… ids (both reset idCounter to 0 and share Date.now()). In a single-process workload this is fine, but in this resilience context multiple agents may crash concurrently. Consider crypto.randomUUID() (Node ≥ 14.17) or appending process.pid to make ids globally unique.

♻️ Suggested change

-const fs = require('fs');
-const path = require('path');
-const EventEmitter = require('events');
+const fs = require('fs');
+const path = require('path');
+const crypto = require('crypto');
+const EventEmitter = require('events');
@@
-let idCounter = 0;
-
 function nextId(prefix) {
-  idCounter += 1;
-  return `${prefix}-${Date.now().toString(36)}-${idCounter.toString(36)}`;
+  return `${prefix}-${Date.now().toString(36)}-${crypto.randomUUID().slice(0, 8)}`;
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.aiox-core/core/resilience/agent-immortality.js around lines 52 - 57, The
current nextId function uses a module-scoped idCounter and Date.now(), causing
possible collisions across processes (e.g., autopsy-…, state-…, reincarnation-…
ids) when agents start in the same millisecond; update nextId to use a
globally-unique source such as crypto.randomUUID() (Node ≥14.17) instead of
relying solely on idCounter/Date.now(), and if crypto.randomUUID() is
unavailable fall back to appending process.pid and a cryptographic random
component (or use crypto.randomBytes) to the generated ID so each process
produces unique IDs; update references to nextId/idCounter accordingly.

---

277-277: 💤 Low value

Minor: simplify reverse-then-find / spread-then-pop.

A few spots can drop the array copy:

• Line 277: [...commits].reverse().find(c => c.agentId === agentId) → commits.findLast(c => c.agentId === agentId)
• Line 299: [...this.list(agentId)].pop() || null → this.list(agentId).at(-1) || null
• Line 463: [...this.listReports()].pop() || null → this.listReports().at(-1) || null

Array.prototype.findLast and at are ES2022, which the codebase targets per AIOX coding standards.

Also applies to: 299-299, 463-463

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.aiox-core/core/resilience/agent-immortality.js at line 277, Replace
reverse-then-find and spread-then-pop patterns with modern ES2022 APIs: for the
variable using commits and agentId (const previous =
[...commits].reverse().find(commit => commit.agentId === agentId)), use
commits.findLast(c => c.agentId === agentId); for the call using
this.list(agentId) ([...this.list(agentId)].pop() || null) use
this.list(agentId).at(-1) || null; and for the call using this.listReports()
([...this.listReports()].pop() || null) use this.listReports().at(-1) || null;
these keep behavior the same while removing unnecessary array copies and relying
on Array.prototype.findLast and Array.prototype.at.

tests/core/resilience/agent-immortality.test.js (1)

44-174: ⚡ Quick win

Solid AC coverage — consider adding a concurrency/persistence-rehydration test.

The suite hits every AC end-to-end and the temp-dir harness is clean. Two optional additions worth considering when the race-condition concern raised on the implementation is addressed:

1. A test that constructs a fresh AgentImmortalityProtocol instance pointing at the same tempDir and calls claimReincarnation() — verifying the queue actually survives across instance boundaries (the persistence story).
2. A test that calls enqueue/claimNext interleaved across two protocol instances on the same tempDir, asserting no duplicate claim. This will fail today (see the RMW race comment) and will serve as the regression guard once locking is added.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/core/resilience/agent-immortality.test.js` around lines 44 - 174, Add
two concurrency/persistence tests: (1) create an AgentImmortalityProtocol
pointing at the same tempDir as an existing protocol, call claimReincarnation()
(or claimNext via ReincarnationQueue) and assert a previously enqueued item is
returned — this verifies the queue rehydrates across instances (reference
AgentImmortalityProtocol, ReincarnationQueue, claimReincarnation, claimNext,
enqueue). (2) Spawn two distinct AgentImmortalityProtocol/ReincarnationQueue
instances pointing at the same tempDir, have one enqueue a report and then
concurrently call claimNext from both instances (or interleave
enqueue/claimNext) and assert only one instance receives the claim and the other
gets null — this will detect RMW race duplication and serve as a regression test
for future locking fixes (reference enqueue, claimNext, QueueStatus).

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes